Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Respect max-substitution-jobs again #11402

Merged
merged 2 commits into from
Sep 3, 2024

Conversation

edolstra
Copy link
Member

@edolstra edolstra commented Sep 2, 2024

Motivation

This broke in #11005. Any number of PathSubstitutionGoals would be woken up by a single build slot becoming available. If there are a lot of substitution goals active, this could lead to us running out of file descriptors (especially on macOS where the default limit is 256).

Note: we have a bit of a thundering herd problem here, since it's pretty inefficient to wake up potentially hundreds of goals, most of which will immediately go to sleep again. Ideally waitForBuildSlot() would wake up only one co-routine of the right type.

Context

Priorities and Process

Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

This broke in NixOS#11005. Any number of PathSubstitutionGoals would
be woken up by a single build slot becoming available. If there
are a lot of substitution goals active, this could lead to us
running out of file descriptors (especially on macOS where the
default limit is 256).
Slight cleanup.
@edolstra edolstra added backport 2.24-maintenance Automatically creates a PR against the branch regression Something doesn't work anymore labels Sep 2, 2024
Copy link

@kh3rld kh3rld left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the issue with file descriptors and the thundering herd problem is crucial. This should help mitigate those issues. 👍

@Ericson2314
Copy link
Member

CC @L-as

@edolstra edolstra merged commit 784a843 into NixOS:master Sep 3, 2024
12 checks passed
@edolstra edolstra deleted the fix-max-substitution-jobs branch September 3, 2024 11:35
edolstra added a commit that referenced this pull request Sep 3, 2024
…1402

Respect max-substitution-jobs again (backport #11402)
@L-as
Copy link
Member

L-as commented Sep 8, 2024

Thanks! Sorry for not noticing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.24-maintenance Automatically creates a PR against the branch regression Something doesn't work anymore
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants